Skip to content

Conversation

@songkant-aws
Copy link
Contributor

@songkant-aws songkant-aws commented Jul 7, 2025

Description

Fix relevance query function over optimization issue in ReduceExpressionsRule

Related Issues

Related to #3834

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

qianheng-aws
qianheng-aws previously approved these changes Jul 7, 2025
* @param isDeterministic Specified isDeterministic flag
* @return Calcite SqlUserDefinedFunction
*/
default SqlUserDefinedFunction toUDF(String functionName, boolean isDeterministic) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you combine the methods toUDF in L35? We don't want to keep diff implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

+ TEST_INDEX_BEER
+ " | STATS count(AcceptedAnswerId) as idCount"
+ " | WHERE simple_query_string(['Tags'], 'taste') and idCount > 200";
+ " | EVAL answerId = AcceptedAnswerId + 1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we change this IT case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I think the previous test case semantic is ambiguous simple_query_string can't query on target field right after an aggregation. Use eval operator to simulate a filter that can't be pushed down makes a lot sense.

}
}

private static RexNode derefMapCall(RexNode rexNode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this method to PlanUtil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Signed-off-by: Songkan Tang <[email protected]>
Signed-off-by: Songkan Tang <[email protected]>
@qianheng-aws qianheng-aws merged commit fed92a3 into opensearch-project:main Jul 9, 2025
27 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 9, 2025
…ceExpressionsRule (#3851)

* Fix RelevanceQueryFunction over optimization issue by ReduceExpressionsRule

Signed-off-by: Songkan Tang <[email protected]>

* Fix PredicateAnalyzerTest

Signed-off-by: Songkan Tang <[email protected]>

* Address comments

Signed-off-by: Songkan Tang <[email protected]>

* Fix spotless check

Signed-off-by: Songkan Tang <[email protected]>

---------

Signed-off-by: Songkan Tang <[email protected]>
(cherry picked from commit fed92a3)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
penghuo pushed a commit that referenced this pull request Jul 9, 2025
…ceExpressionsRule (#3851) (#3858)

* Fix RelevanceQueryFunction over optimization issue by ReduceExpressionsRule



* Fix PredicateAnalyzerTest



* Address comments



* Fix spotless check



---------


(cherry picked from commit fed92a3)

Signed-off-by: Songkan Tang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@songkant-aws songkant-aws deleted the relevance-query-fix branch August 1, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-dev bug Something isn't working calcite calcite migration releated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants